Repository navigation
feat(lab): DiffViewer — unified/split file diff for code review - #7536
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
github-actions
Bot
added
community
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsDiffViewer (@astryxdesign/lab) · View in Storybook
Bundle Size Summary
Accessibility AuditStatus: No accessibility violations detected. Visual Regression38 of 474 shot(s) changed. A change here is a question, not a failure: check whether the after is the
and 18 more. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for the thorough Lab DiffViewer; changes are requested before it can merge: two behavior fixes and a scope split.
- Line-number keyboard handling is attached to the whole table, so ArrowUp/ArrowDown/Home/End pressed inside an annotation row (for example a comment textarea) move focus to a line number; please handle those keys only when a line-number button has focus, and add a test.
parseUnifiedDiffsplits thediff --githeader at the first space, so a binary or mode-only change to a path containing a space shows the wrong path and a false Renamed badge.- At about 3,100 authored lines, please move the standalone
createDiffFileutility (with its tests and story) to a follow-up and split further, or explain why the remaining pieces must land together.
[Automated review]
| onKeyDown: | ||
| onLineNumberClick != null ? handleLineButtonsKeyDown : undefined, |
There was a problem hiding this comment.
This key handler is on the wrapper around the whole table, annotation rows included, so ArrowUp/ArrowDown/Home/End pressed inside renderLineAnnotation content (a comment textarea, a Resolve button) move focus to a line-number button and cancel the caret move. Consider delegating only when the event comes from a line-number button, for example if ((event.target as Element).closest('[data-diff-line-button]')) handleLineButtonsKeyDown(event);, with a test that types in an annotation textarea.
| } from './diffTypes'; | ||
|
|
||
| const HUNK_HEADER = /^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@/; | ||
| const GIT_HEADER = /^diff --git (?:"?a\/)?(.+?)"? (?:"?b\/)?(.+?)"?$/; |
There was a problem hiding this comment.
The lazy groups split the header at the first space: diff --git a/assets/my logo.png b/assets/my logo.png gives assets/my and logo.png b/assets/my logo.png. For binary or mode-only changes nothing overrides these paths, so finalize reports the file as renamed. Splitting an unquoted header at the midpoint when both halves match (or reading the Binary files ... differ line) would fix it, and a test with a spaced path would cover it.
There was a problem hiding this comment.
Thanks for the fixes: annotation-row keys and paths with spaces now behave, but changes are still requested before this Lab DiffViewer can merge.
- The clickable line numbers share one roving Tab stop with ArrowUp/ArrowDown/Home/End, while the accessibility spec (AST-020 FR15) keeps independent controls like these in the page Tab order unless the component's record names an exception, so please give each line number its own Tab stop or have a maintainer record that exception.
- At about 2,750 authored lines, please stack
parseUnifiedDiff(with its tests, stories and showcase) as the next layer, or explain concretely why it must land with the viewer. parseUnifiedDiffkeeps git's escapes in quoted non-ASCII paths (for examplecaf\303\251.md), so please decode quoted paths and add a test.
[Automated review]
| itemSelector: '[data-diff-line-button]', | ||
| orientation: 'vertical', | ||
| wrap: false, | ||
| hasRovingTabIndex: true, |
There was a problem hiding this comment.
This gives the clickable line numbers one roving Tab stop with ArrowUp/ArrowDown/Home/End. AST-020 FR15 keeps a set of independently focusable controls in the page Tab order, without arrow keys, unless the component's record names an exception, and these buttons are independent: each starts its own line's action and nothing around them is a composite widget. Either drop the roving stop so each line number keeps its own Tab stop, or have a maintainer record the gutter as a named FR15 exception in a DiffViewer record.
| function cleanPath(raw: string, prefix: 'a/' | 'b/'): string | null { | ||
| let path = raw.split('\t')[0].trim(); | ||
| if (path.startsWith('"') && path.endsWith('"') && path.length >= 2) { | ||
| path = path.slice(1, -1); |
There was a problem hiding this comment.
Git quotes non-ASCII paths by default ("a/docs/caf\303\251.md"), and only the quotes are removed here, so the header shows docs/caf\303\251.md. Decoding the C-style escapes (octal byte runs as UTF-8, plus \\, \", \t, \n) here and in the quoted diff --git fallback, with a test, would show docs/café.md.
The structured model a diff viewer renders (files, hunks, numbered lines) and a tolerant parser for git and diff -u patches: multi-file patches, mode, rename and binary lines, bare hunks, CRLF and no-newline markers. Paths with spaces are read from the git header by its middle space, and git-quoted paths are decoded (octal UTF-8 bytes and C escapes). It never throws. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
Renders one DiffViewerFile: a file header (path, Added/Deleted/Renamed token, +/- counts, a headerEnd slot, an optional disclosure) over its hunks in unified or split view, with old/new line numbers, wrapping, maxHeight and a per-instance syntax theme. Rows paint from the same syntax-theme diff slots as CodeBlock. renderLineAnnotation adds a full-width row under any line, and onLineNumberClick makes each line's anchor-side number a button labelled with the line it reports; each is its own Tab stop (AST-020 FR15). Changed lines carry a visually hidden Added/Removed label, and hidden column headers name every column. Adds tests, docs, a docsite showcase block, Storybook stories, the registry route lock entries, and the @astryx.diffViewer.* en keys. Co-authored-by: Cindy Zhang <cindyxz@meta.com>
9407bc6 to
939219d
Compare
There was a problem hiding this comment.
This Lab DiffViewer looks ready to merge into its parser branch: each clickable line number is now its own Tab stop, the parser, createDiffFile and the performance page live in their own stacked pull requests, and the viewer's tests, lint, catalog, registry-lock and accessibility checks pass on this revision, with its files type-checking cleanly. No further changes are needed here; the stack reaches main once #7568 lands. One optional follow-up: renderLineAnnotation draws an empty row when the callback returns false (for example cond && <Thread />), so treating booleans like null would make that idiom safe.
[Automated review]
| {renderMarker(row.cell, 'm')} | ||
| {renderCode(row.cell, 'c')} | ||
| </tr> | ||
| {note != null && ( |
There was a problem hiding this comment.
Optional, not blocking: false is a valid ReactNode (for example from ref => hasThread(ref) && <Thread />), but it passes this != null test and draws an empty, padded annotation row under every such line; the split branch (hasNote, line 893) has the same test. The JSDoc asks for null, so documented usage is fine, but treating booleans and '' like null here, with a test, would make the common JSX idiom safe.
…lines, and reads copy lines Two different paths with no rename lines (diff -u a.orig a, diff -ruN old new, git diff --no-index, a copy) were reported as renames. They now read as modified with both paths kept. Copy from/copy to lines set the paths, so a copy whose paths contain spaces no longer takes the header's first-space split.
…n returns false `hasThread && <Thread />` returns false for most lines; React renders nothing for it, but the viewer still drew an empty bordered row under each of those lines. Booleans now count as nothing to show, like null.
…del (#7568) <!-- Use this template only when adding a new user-facing capability is the primary intent. --> ## User need Builders showing code changes (review pages, deploy diffs, proposed-edit panels) usually start from a patch: `git diff` output, a `diff -u` file, or a hunk from an API. To render it they need it as structured files, hunks and numbered lines, without writing their own parser. ## Problem and solution fit - **Primary:** `parseUnifiedDiff(patch)` in `@astryxdesign/lab` returns a `DiffViewerFile[]`, one per file in the patch. It handles: - git patches with mode, rename and binary lines; - classic `diff -u` output and multi-file patches; - bare hunks, CRLF line endings and no-newline markers. It never throws. - **Paths:** - An unquoted git header path with spaces (`a/my logo.png b/my logo.png`) is read by its middle space when both sides match. - Git-quoted paths are decoded, so `"a/docs/caf\303\251.md"` reads `docs/café.md`. Decoding covers octal UTF-8 bytes and C escapes, in headers, `---`/`+++` lines and rename lines. - **Supporting:** the model types (`DiffViewerFile`, `DiffViewerHunk`, `DiffViewerLine`, …) and 13 tests. This is the bottom layer of the Lab DiffViewer stack, split out at review. #7536 (the viewer) stacks on it; its tests, stories and showcase render parsed patches. ## Current specification None for Lab (by design). No Core change. ## Public delta and caller burden New Lab-only exports: `parseUnifiedDiff` and the `DiffViewer*` model types. ```ts const files = parseUnifiedDiff(patch); ``` ## Design and alternatives - **Tolerant, not strict:** unknown header lines are skipped, and a hunk whose counts run short ends where its lines end. Patches pasted from tools and chat are often trimmed. - **Line anchors:** each line has old/new numbers, so a comment anchor survives a change of layout. ## Evidence Vitest, `parseUnifiedDiff.test.ts`, 13 tests: - patch shapes: git, multi-file, rename, binary, deleted, `diff -u`, bare hunk; - line edge cases: `--`-prefixed removed lines, CRLF, no-newline markers, non-diff input; - paths: spaced paths in headers and renames, and quoted non-ASCII and escaped paths. ## Scope - [x] One capability is added. - [x] Unrelated fixes, cleanup, and policy changes were removed or split. - [x] Consumer docs, focused tests, and migration evidence ship with the feature. - [x] Public text and artifacts contain no internal Meta context. ## Testing ``` pnpm vitest run packages/lab/src/DiffViewer # 13 passed ``` Originally written by @latxh in #7536.
# Conflicts: # packages/lab/src/DiffViewer/diffTypes.ts # packages/lab/src/DiffViewer/index.ts # packages/lab/src/DiffViewer/parseUnifiedDiff.ts # packages/lab/src/index.ts
There was a problem hiding this comment.
Changes are requested on this Lab DiffViewer for one right-to-left fix; everything else checks out on this revision. On a right-to-left page, when any line is wider than the viewer (the default unwrapped mode), it opens scrolled to the lines' ends, so each code line's start is cut off and short lines look blank; for example, the Split story in a right-to-left frame at 1280 px hides the start of 23 of its 44 non-empty code cells, while left-to-right hides none. Please keep line starts visible at the initial scroll position (for example a left-to-right viewport like a code editor, or sticky mirrored gutters) and add a right-to-left story or check with a line wider than the viewer. The small line-number targets and the folder-level 'use client' noted earlier remain optional follow-ups.
[Automated review]
| // Code, hunk headers, paths, and counts read left-to-right in every page | ||
| // direction; only the table's column order (gutters) mirrors under RTL. | ||
| ltr: { | ||
| direction: 'ltr', | ||
| unicodeBidi: 'isolate', | ||
| }, |
There was a problem hiding this comment.
On a right-to-left page these left-to-right code cells sit inside a max-content table whose viewport inherits the page direction, so when any line is wider than the viewer it opens at its right edge and every line's start is cut off (short lines look blank). In Chromium, the Split story at 1280 by 800 hides the start of 23 of its 44 code cells right-to-left and none left-to-right. Keeping the hunk viewport left-to-right, or making the mirrored gutters sticky and opening at the code's start, would keep line starts and numbers visible together; a right-to-left story with a long line would guard it.
… pages On a right-to-left page the hunk table mirrored, so a line wider than the viewer opened scrolled to the lines' ends: every line's start was cut off and short lines looked blank. The hunk area now reads left-to-right in every page direction, as code editors do (line numbers on the left, old file on the left in split view), while the file header and annotation rows (the caller's comments) follow the page. Adds a RightToLeft story with lines wider than the viewer and an RTL annotation.
There was a problem hiding this comment.
Changes are requested on this Lab DiffViewer for one right-to-left follow-up: code lines now open at their start on right-to-left pages, but annotation rows were switched back to right-to-left inside that left-anchored hunk area. Whenever a line is wider than the viewer, a right-to-left page's comments now open past the right edge (in the Review Comments split view the comment text is out of view while its Resolve button shows, and the new Right To Left story's note opens off-screen), and the split divider in annotation rows moves to the outer left edge. Please keep annotation content in view at the initial scroll, for example by leaving the cells left-to-right and giving an inner wrapper the page direction and the viewer's visible width, and extend the right-to-left story or a check so its note is visible on load. The small line-number targets and the folder-level 'use client' remain optional follow-ups.
[Automated review]
| // Annotation rows hold the caller's content (review comments), which reads | ||
| // in the page's direction even inside the left-to-right hunk area. | ||
| annotation: { | ||
| direction: {default: 'ltr', ':is([dir="rtl"] *)': 'rtl'}, |
There was a problem hiding this comment.
On a right-to-left page this switches the annotation cell back to right-to-left inside the left-to-right viewport, so caller content starts at the cell's right end, which is the far end of the table. Whenever any line is wider than the viewer, that end is out of view at the initial scroll: in the Review Comments split view the comment text sits past the right edge while its Resolve button shows, and the Right To Left story's note opens more than 2,000 px off-screen. It also moves codeOldSide's borderInlineEnd on the old-side annotation cell (line 924) to the outer left edge. Leaving the cell left-to-right and giving an inner wrapper the page direction and the viewer's visible width would keep comments in view and the divider in place.
…t pages Annotation cells were switched to right-to-left inside the left-to-right hunk area, so on a wide table a right-to-left page's comments started at the table's far right end, out of view, and the split divider moved to the outer edge. The cells now stay left-to-right; an inner wrapper takes the page direction and is capped at the viewport's visible width, which the shared ResizeObserver writes to a custom property on the viewport (no re-render). The RightToLeft story's note now sits on the old side, where it opens in view.
There was a problem hiding this comment.
Changes are requested on this Lab DiffViewer for one remaining right-to-left case: the new annotation wrapper keeps comments in view in unified view and on the old side of split view, but a new-side comment in split view still opens with its start past the right edge whenever that side starts inside the viewer. In the Review Comments story's split view on a right-to-left page, the comment text sits just past the edge of the 1,046 px viewer while its Resolve button is visible, as on the previous revision. Please cap the new-side wrapper to the part of the viewer that side occupies (for example by subtracting how far into the viewer the new side starts) and add a right-to-left split-view story or check with a new-side note. Optionally, overflowWrap: 'anywhere' on the wrapper would stop unbreakable right-to-left text wider than the cap from being clipped at the table's left edge.
[Automated review]
| {newNote != null && ( | ||
| <div {...stylex.props(annotationStyles.content)}> | ||
| {newNote} | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
On a right-to-left page this new-side wrapper gets the same cap as the old side, the viewer's whole visible width, but it starts where the new-side cell starts, mid-viewer, so it still runs past the right edge and right-to-left content starts out of view. In the Review Comments split view the comment text sits just past the edge of the 1,046 px viewer while its Resolve button shows, as on the previous revision. Capping this wrapper to the part of the viewer the new side occupies (for example, the observer also writing how far into the viewer the new side starts, or 0 when it starts past the edge) would keep its start in view, and a right-to-left split-view story or check with a new-side note would guard it.
| const annotationStyles = stylex.create({ | ||
| content: { | ||
| direction: {default: null, ':is([dir="rtl"] *)': 'rtl'}, | ||
| maxInlineSize: `calc(var(${VISIBLE_INLINE_SIZE}, 100%) - 2 * ${spacingVars['--spacing-3']})`, |
There was a problem hiding this comment.
Optional: with the page-direction wrapper narrower than the table, an unbreakable right-to-left run wider than this cap overflows toward the table's left edge, which the left-to-right viewer cannot scroll past (a 1,178 px token in the Right To Left note starts 232 px left of the viewer while the scroll is already at its start). Adding overflowWrap: 'anywhere' here would let such runs wrap within the cap.
…art of the viewer that side occupies The new-side annotation got the whole visible width as its cap but starts mid-viewer, so on a right-to-left page its start ran past the right edge. The ResizeObserver (now also watching the content box, which resizes with the columns) writes how much of the visible width the new side occupies, measured from the new side's first column header; a slice too narrow to hold a word counts as past the edge. Annotation content also wraps long unbreakable runs (overflow-wrap: anywhere). Adds a RightToLeftSplitComments story with a comment on each side.
There was a problem hiding this comment.
Changes are requested on this Lab DiffViewer for one follow-up to the new-side cap: right-to-left comments in split view now open in view, but when only a narrow part of the new side is visible the cap crushes the comment on both left-to-right and right-to-left pages. In the Review Comments story's split view at a 768 px page, the comment card is 70 px wide, its text stacks one letter per line and the Resolve button is a 28 px sliver with its label clipped, while the previous revision kept that card intact on left-to-right pages. Please treat a slice narrower than a usable comment width as past the edge (for example by raising MIN_NEW_SIDE_VISIBLE_PX to around 200–240 px, so narrower slices scroll as they already do under 48 px) and add a story or check with a narrow new-side slice. Optionally, measuring where the new side starts in layout pixels (such as the header cell's offsetLeft) instead of getBoundingClientRect() would keep the cap right inside scaled containers, such as a dialog during its opening animation.
[Automated review]
| * the new side counts as starting past the edge (its note then needs a scroll, | ||
| * like its code). | ||
| */ | ||
| const MIN_NEW_SIDE_VISIBLE_PX = 48; |
There was a problem hiding this comment.
Blocking: with a 48 px floor, a new-side slice only somewhat wider than that still caps the comment at the slice, whatever the content needs. In the Review Comments story's split view at a 768 px page the cap is 70 px: the comment text stacks one letter per line and Resolve shrinks to a 28 px sliver with its label clipped, on left-to-right pages too. Raising this floor to a usable comment width (around 200–240 px) would send narrower slices to the existing fallback (the whole visible width, scrolled to like that side's code).
| const start = | ||
| newSide.getBoundingClientRect().left - | ||
| table.getBoundingClientRect().left; |
There was a problem hiding this comment.
Non-blocking: getBoundingClientRect() includes ancestor transforms while viewport.clientWidth does not, so inside a scaled container the cap comes out too wide; with the story scaled to 50% and resized, it read 710 px instead of 422 px and the right-to-left note started out of view. A dialog's scale(0.95) opening animation could do the same if the first measurement lands during it. Measuring in layout pixels, for example the header cell's offsetLeft within the table, avoids this.
…ead of crushing its comment With a 48 px floor, a new-side slice only a little wider capped the comment at the slice: at a 768 px page the Review Comments card was 70 px wide, its text one letter per line and its Resolve button clipped, on left-to-right pages too. The floor is now 240 px (a usable comment with its actions); narrower slices keep the whole visible width and are reached by scrolling, like that side's code. Where the new side starts is now read in layout pixels (the header cell's offsetLeft) so a scaled ancestor, such as a dialog's opening animation, no longer skews the cap.
There was a problem hiding this comment.
Changes are requested on this Lab DiffViewer for one follow-up to the new-side annotation cap: the 240 px floor fixes the crushed comment card (at a 768 px page the Review Comments card is now whole on both page directions), but the cap is only re-measured on resize, so switching from unified to split view on a newly added file with a long line leaves the new-side comment uncapped. In Chromium that comment then spans 2,499 px, the full width of the table's new side: on a right-to-left page its text opens off-screen, and on a left-to-right page its Resolve button sits about 1,450 px past the viewer's edge, until a resize corrects both. Please re-measure when the table's layout changes (for example by also observing the table, or re-attaching the ref when viewMode or hasLineNumbers changes), fall back to the visible width rather than 100% for the new side, and add a story or check that switches to split view on an added file.
[Automated review]
| const content = viewport.firstElementChild; | ||
| const stops = [observeResize(viewport, measure)]; | ||
| if (content != null) { | ||
| stops.push(observeResize(content, measure)); | ||
| } |
There was a problem hiding this comment.
Blocking: these two observers are the only re-measure triggers, and neither fires when the table's columns change without a row-height change, because the content box is a plain block that always takes the viewport's width. Switching from unified to split view on an added-only diff (any new file) therefore leaves --diff-viewer-new-side-visible-inline-size removed, as unified view left it, and the new-side wrapper falls back to 100% of its cell (line 646). In the Review Comments story with a 16-line added file at a 1,280 px page, the comment wrapper is 2,499 px after “Split view”: right-to-left text at 2,591–2,755 px in a 185–1,231 px viewer, left-to-right Resolve at 2,543–2,619 px in a 49–1,095 px viewer; a 1 px resize sets the cap to 975 px and both come into view. Also observing the <table> (2,580 px unified, 2,618 px split here) or re-attaching the ref when viewMode or hasLineNumbers changes, plus var(--diff-viewer-new-side-visible-inline-size, var(--diff-viewer-visible-inline-size, 100%)) at line 646 and a story or check that switches view on an added file, would cover it.
| * the edge: its note keeps the whole visible width and is reached by | ||
| * scrolling, like its code. | ||
| */ | ||
| const MIN_NEW_SIDE_VISIBLE_PX = 240; |
There was a problem hiding this comment.
Non-blocking: just above this floor the story's own card still squeezes its action. At a 920 px page the slice is 246 px, the wrapper 222 px, the text wraps to two lines and Resolve renders as “Res…” (64 px instead of 75) on both page directions. It stays operable and named; a somewhat higher floor would keep this card whole.
… columns change The cap was measured only on resize, and a unified-to-split switch on an added file changes the columns without resizing any observed box, so the new-side comment was left uncapped (2,499 px wide: its text or its actions off-screen). The table is now observed as well, the viewport ref re-attaches (measuring again) when viewMode, hasLineNumbers, isWrapped or file change, and the new-side cap falls back to the visible width rather than 100%. The narrow-slice floor rises to 320 px so the story's comment card keeps its action whole. Adds a ReviewCommentsOnAddedFile story.
…oll with the code Drops the measured annotation caps (the visible-width custom properties, the ResizeObserver, the new-side header marker and the narrow-slice floor). Annotation rows stay in the left-to-right hunk area at the full width of their side and scroll with the code, as in a code editor. The content still reads in the page direction and wraps long unbroken words. The RightToLeft story now shows a comment on each side; the stories that only covered the measured caps are removed.
There was a problem hiding this comment.
Changes are requested on this Lab DiffViewer for one annotation-width fix; the simplified comment placement described in the pull request is judged as intended, and nothing else blocks. In the default unwrapped view a comment wider than the code sets the whole table's width and stays on one line: in the Review Comments story a 438-character comment stretches the table from 1,046 to 2,986 px, so it must be scrolled sideways to read and its Resolve button ends up about 1,840 px past the edge, on both page directions and in both view modes. Please keep annotation content from widening the table so it wraps within its side (for example contain: 'inline-size' on the annotation content wrapper, which wraps that comment to four lines at the original width), and add a story or test that renders a multi-sentence comment.
[Automated review]
| // The caller's content (review comments) reads in the page's direction. Its | ||
| // row stays in the left-to-right hunk area and scrolls with the code, as in a | ||
| // code editor; long unbroken words wrap instead of widening the table. | ||
| const annotationStyles = stylex.create({ | ||
| content: { | ||
| direction: {default: null, ':is([dir="rtl"] *)': 'rtl'}, | ||
| overflowWrap: 'anywhere', | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Blocking: overflowWrap: 'anywhere' lowers only this wrapper's minimum width. The table around it is width: max-content (lines 483–486), so a comment wider than the code still sets the table's width and stays on one line, contrary to the comment above: in the Review Comments story a 438-character comment stretches the table from 1,046 to 2,986 px (1,236 to 3,562 px in split view). Keeping this content out of the table's intrinsic width, for example contain: 'inline-size' here, wraps it within the width the code sets (four lines at 1,046 px in the same Chromium probe); a story with a multi-sentence comment would keep it covered.
The unwrapped table is max-content wide, so a comment longer than the code set the table's width and stayed on one line, scrolled sideways with its actions off-screen. The annotation content now has inline-size containment, so it no longer counts toward the table's intrinsic width and wraps within the width the code sets. Adds a LongComment story.
There was a problem hiding this comment.
This Lab DiffViewer looks ready to merge: long review comments now wrap within the width the code sets instead of widening the table (checked in both view modes and page directions, and covered by the new Long Comment story), and the viewer's tests, lint, typecheck, catalog, registry-lock and accessibility checks pass on this revision. Nothing else is needed here; the small line-number targets, the folder-level 'use client' and empty-string annotations remain optional follow-ups before promotion.
[Automated review]
github-actions
Bot
removed
the
needs:code-review
…of a text (#7565) <!-- Use this template only when adding a new user-facing capability is the primary intent. --> ## User need Some callers of the Lab `DiffViewer` hold two versions of a file, not a patch: an editor's saved and unsaved text, an agent's proposed edit, two config revisions. Today they would have to produce a unified diff themselves before `parseUnifiedDiff` can read it. ## Problem and solution fit - **Primary:** `createDiffFile({oldValue, newValue, path | oldPath/newPath, contextLines})` in `@astryxdesign/lab` returns the same `DiffViewerFile` that `parseUnifiedDiff` produces. - It computes a line-level Myers diff with configurable context (default 3) and splits hunks the way `diff -u` does. - It derives added, deleted and renamed status from a `null` or differing path. - **Supporting:** tests, a Storybook story (`Lab/DiffViewer` → From Two Texts) and a doc example. Split out of #7536 at review so the viewer and this utility can be reviewed separately. Originally written by @latxh as part of that PR. ## Current specification None for Lab (by design). No Core change. ## Public delta and caller burden New Lab-only export `createDiffFile` and type `CreateDiffFileOptions`. ```tsx const file = createDiffFile({oldValue: before, newValue: after, path: 'src/app.ts'}); <DiffViewer file={file} /> ``` ## Design and alternatives - **Bounded memory:** shared leading and trailing lines match without a search, and the minimal search stops after 2,000 edits. A bigger change shows as one replaced block instead of a minimal diff. A fully rewritten 6,000-line file takes about 42 ms and 201 MB peak, against 1.2 s and 1.36 GB unbounded. - Rejected: parallel `oldValue`/`newValue` props on `DiffViewer` (two inputs with override rules). One model type, two builders. ## Evidence - Vitest: `createDiffFile.test.ts` covers identical text, a minimal edit with numbering and header, a round trip of both texts, hunk splitting, added/deleted/renamed status, a minimal diff between long shared runs, the bounded fallback round trip, a 150,000-line text changed at both ends, and `NaN` context. ## Scope - [x] One capability is added. - [x] Unrelated fixes, cleanup, and policy changes were removed or split. - [x] Consumer docs, focused tests, and migration evidence ship with the feature. - [x] Public text and artifacts contain no internal Meta context. ## Testing ``` pnpm vitest run packages/lab/src/DiffViewer # 49 passed (10 createDiffFile + 20 viewer + 19 parser) ``` #7568 and #7536 are on main; this PR targets main.
## Maintainer impact The Lab DiffViewer has no way to measure how it behaves on the change sets people actually review: many files and a lot of code. Anyone tuning it (virtualization, lazy file bodies, highlighting) needs one runnable page that reproduces those workloads and reports the same numbers every time. ## Intended invariant Sandbox-only. No package API, component behavior or visual change. Modeled on the existing CodeBlock and Markdown performance pages. ## Change - `apps/sandbox/.../pages/diffviewer-perf`: renders a whole change set, one `DiffViewer` per file, inside a fixed-height scrollable surface. - Deterministic workloads (`benchmark.ts`, seeded, byte-identical every run): - **many files:** 50 / 200 / 500 / 1,000 files at about 10k / 50k / 100k / 150k lines. Mostly small files, some medium ones and a few very large ones, with sparse edits plus a few added, deleted and fully rewritten files; - **one file:** 100 / 1,000 / 5,000 / 20,000 lines, sparse edits or a full rewrite. - Controls: unified or split, syntax highlighting on or off. - Metrics: - render: diff model (`createDiffFile`) time, first file on screen, full render to the next painted frame, long tasks (count and total), DOM nodes, heap growth (where the browser reports it); - scroll: a top-to-bottom test over the whole list (fps, dropped frames, long tasks); - interactions: timed collapse all, expand all, one file, view switch and syntax switch. Stacked on #7565 (createDiffFile); #7568 (the parser) and #7536 (the viewer) are on main. Merge order: #7565, then this. ## Evidence Production Sandbox build of this branch (`pnpm build`, static export verified), headless Chrome 155, 200 files (50,827 file lines, 16,465 diff rows), unified, highlighted: | Metric | Value | | --- | --- | | Diff model (`createDiffFile`) | 93 ms | | First file on screen / full render | 4.9 s / 5.7 s | | Long tasks | 3, 5.8 s total | | DOM nodes / heap growth | 203,986 / 163 MB | | Scroll test (fps / dropped / long tasks) | 47 / 49 / 1 | | Collapse all / expand all / switch to split | 0.7 s / 4.0 s / 6.8 s | Every control and button was driven in the browser; no console errors. In a bare page with no app shell, the same Lab build renders 50 / 200 / 500 / 1,000 files in 1.3 / 3.1 / 5.2 / 8.3 s, each as a single long task. Profiling the 200-file case puts most of that time in per-file scroll-area measurement (forced style and layout) inside the one commit. The diff model takes about 80 ms and tokenizing about 70 ms. That is the baseline this page exists to move. ## Scope - [x] No intended public API, product behavior, visual direction, or policy change. - [x] Product changes discovered during the work were removed or split. - [x] Public text and artifacts contain no internal Meta context. ## Testing - `vitest run apps/sandbox/.../diffviewer-perf`: 7 passed (deterministic workloads, size targets within 5%, small/large mix, added/deleted/rewritten files) - ESLint and Prettier clean - Sandbox production build + static-export verification (adds the route to `scripts/next-export-routes.json`) - Real Chromium against that build (see Evidence)
This branch was successfully deployed
2 participants
User need
Builders of code review and change-inspection tools (PR review pages, deploy diffs, config change audits, agent "proposed edits" panels) need to show a file's changes the way reviewers expect to read them: per-file header with path and +/− counts, old/new line numbers, unified or side-by-side layout, and comment threads anchored under specific lines.
CodeBlockwithisDiff(#7389) answers the inline case — a diff fence in prose or chat — but by design it is a single-column code surface over the patch text: one line-number column, no file header, no old/new numbering, no split view, and no way to put content between lines. Composing those on top ofCodeBlockmeans re-implementing the row model, numbering and pairing in every product, which is the work a design system should own once.Lab is the staging area for this: canary-only, no stability promise, iterate freely before a Core spec.
Problem and solution fit
DiffViewerin@astryxdesign/lab— renders oneDiffViewerFile:old → newon rename), Added/Deleted/RenamedToken(a category of change, not an alert — so not a statusBadge), +N −M counts, aheaderEndslot, and an optional disclosure (controlled or uncontrolled);viewMode="unified"or"split"; split pairs each run of removed lines with the run of added lines that replaced it, row by row;hasLineNumbers), wrapping (isWrapped),maxHeight, per-instancesyntaxTheme;renderLineAnnotation(ref)— a full-width row under any line (review comments); in split view the row sits under the line's own side;onLineNumberClick(ref, event)— turns line numbers into buttons (e.g. "start a comment here"). Only the line's anchor-side number is a button, and each is its own Tab stop (AST-020 FR15: independent controls get no arrow-key movement).parseUnifiedDifflanded in feat(lab): parseUnifiedDiff reads a patch into the DiffViewer file model #7568. The two-text buildercreateDiffFilefollows in feat(lab): createDiffFile builds a DiffViewer file from two versions of a text #7565.packages/lab/blocks/DiffViewerShowcase), StorybookLab/DiffViewer, the registry route lock entries,@astryx.diffViewer.*en catalog keys (+ core changeset for the catalog).Paint reuses CodeBlock's diff work rather than inventing a parallel palette: rows read the five syntax-theme diff slots (
--color-syntax-added,-removed,-diff-header,-added-marker,-removed-marker) and tokens come from the shared tokenizer/astryx-token-*classes, so one syntax theme styles both components.Current specification
None for Lab (by design).
component:CodeBlockDEC-1 rejects "a separate Diff component" for diff fences (a fence is a CodeBlock); this component is a different fact boundary — a file-level review surface with old/new numbering, split layout, file header, and between-line content — and does not change how CodeBlock or Markdown render fences. A Core spec would come with promotion.Public delta and caller burden
New Lab-only exports; no Core component API changes. Core gains only new
encatalog keys (no existing string changes).Defaults:
viewMode='unified',hasLineNumbers=true,isWrapped=false,isCollapsible=true(needs a path to name the control),defaultIsCollapsed=false; language inferred from the file extension, else plain text.Line anchors: every line has exactly one
DiffViewerLineRef(side+lineNumber), identical in unified and split view — added →new, removed →old, context →new— so a comment anchor never moves when the reader switches layouts.Theming targets:
astryx-diff-viewer(reflectsviewMode),astryx-diff-viewer-header,astryx-diff-viewer-path.Design and alternatives
DiffViewerthat owns the list.file) plus builders — rejected: paralleldiffstring /files/oldValue+newValueprops (conditional overrides between inputs).<table>with per-side gutter/marker/code cells) — gives split view, between-line annotation rows (colSpan) and wrapped rows that keep gutters aligned. Rejected: extending CodeBlock's per-line grid (it has no second number column or between-line rows).+/-glyph (hidden from AT) plus a visually hidden, localized "Added"/"Removed" label; header counts are announced as "N lines added, M lines removed".Evidence
packages/lab/src/DiffViewer: 20 component tests (unified/split row model, gutters off, collapse controlled + uncontrolled witharia-expanded/aria-controls, annotations in both views, line-number buttons on the anchor side only, each labelled with the line it reports, every line number its own Tab stop, named columns, badges/rename path, binary/no-change messages,headerEnd, ref/className/DOM passthrough +data-view-mode).Splitstory under RTL cut off the start of 23 of 44 code lines before this and none after. The file header and comment text follow the page. Comment rows are deliberately simple in this Lab version: they belong to the code layout, take the full width of their side, and scroll with the code, so on a wide right-to-left diff a comment may need a horizontal scroll to bring into view (theRightToLeftstory shows this).Unified (default)
Split, light
Split, dark
Split, wrapped (dark)
Multi-file patch: modified, added, renamed, binary
Review comments in split view (annotation rows on their own side)
Scope
Testing
Storybook:
Lab/DiffViewer— Default, Split, Wrapped, Right To Left, Multi File Patch, Viewed Toggle, Review Comments, Long Comment, Compact.